wip: #268 #270
wip: #268 #270
Conversation
|
@sergey-tihon Thanks for the help so far! I got up to running tests on the new controllers, but now another test fails that I haven't touched... I'm very new to the F# ecosystem - sorry I'm probably missing something obvious :) |
|
it was not an obvious issue... i believe you found another but with caching of provided types. the next step would be to handle response correctly, current implementation tries to deserialize response as json here https://github.com/fsprojects/SwaggerProvider/blob/master/src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs#L361 |
|
|
||
| Some(MediaTypes.ApplicationOctetStream, ty) | ||
| | content when content.Keys |> Seq.exists MediaTypes.isTextMediaType -> | ||
| let textKey = content.Keys |> Seq.find MediaTypes.isTextMediaType |
There was a problem hiding this comment.
here you can avoid code duplication and double enumeration.
This is perfect place to use F# Active Patterns, you can refactor isTextMediaType to active pattern that match and return matched value in the same time
There was a problem hiding this comment.
Thanks for the tips ❤️! As you can guess, I'm not quite F#luent yet...
I got this to a point where the text/plain test passes but text/csv does not and I can't figure out why... I'm so lost as I can't just run the test in debug mode and use breakpoints in Rider 😅
There was a problem hiding this comment.
i think that this log line is the key (aspnetcore does not know how to format csv)
warn: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor[2]
No output formatter was found for content types 'text/csv, text/csv' to write the response.
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for text-based media types (text/plain, text/csv) in the Swagger provider by adding new controllers, test coverage, and compilation logic for handling text responses.
- Adds controllers that return text/plain and text/csv content types
- Implements custom CSV output formatter for ASP.NET Core
- Updates the operation compiler to handle text/* media types as string responses
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Swashbuckle.WebApi.Server/Swashbuckle.WebApi.Server.fsproj | Adds new ReturnTextControllers.fs to project compilation |
| tests/Swashbuckle.WebApi.Server/Startup.fs | Registers CSV output formatter with MVC options |
| tests/Swashbuckle.WebApi.Server/Controllers/ReturnTextControllers.fs | New controllers for text/plain and text/csv endpoints with custom formatter |
| tests/SwaggerProvider.ProviderTests/v3/Swashbuckle.ReturnTextControllers.Tests.fs | Test cases for the new text-based endpoints |
| tests/SwaggerProvider.ProviderTests/SwaggerProvider.ProviderTests.fsproj | Adds new test file to project compilation |
| src/SwaggerProvider.Runtime/RuntimeHelpers.fs | Removes extra blank line |
| src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs | Adds text media type detection and string response handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tests/Swashbuckle.WebApi.Server/Controllers/ReturnTextControllers.fs
Outdated
Show resolved
Hide resolved
| [<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/plain")>] | ||
| member this.Get() = | ||
| "Hello world" |> ActionResult<string> | ||
|
|
||
| [<Route("api/[controller]")>] | ||
| [<ApiController>] | ||
| type ReturnCsvController() = | ||
| [<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/csv")>] |
There was a problem hiding this comment.
Using Consumes attribute for a GET request that doesn't accept a request body is unnecessary and potentially confusing. GET requests typically don't consume request bodies.
| [<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/plain")>] | |
| member this.Get() = | |
| "Hello world" |> ActionResult<string> | |
| [<Route("api/[controller]")>] | |
| [<ApiController>] | |
| type ReturnCsvController() = | |
| [<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/csv")>] | |
| [<HttpGet; Produces("text/plain")>] | |
| member this.Get() = | |
| "Hello world" |> ActionResult<string> | |
| [<Route("api/[controller]")>] | |
| [<ApiController>] | |
| type ReturnCsvController() = | |
| [<HttpGet; Produces("text/csv")>] |
tests/SwaggerProvider.ProviderTests/v3/Swashbuckle.ReturnTextControllers.Tests.fs
Outdated
Show resolved
Hide resolved
…ers.fs Co-authored-by: Copilot <[email protected]>
…ontrollers.Tests.fs Co-authored-by: Copilot <[email protected]>
2 participants
No description provided.